Skip to content

refactor(ext/node): consolidate node:fs (part 9)#32659

Merged
bartlomieju merged 7 commits intodenoland:mainfrom
bartlomieju:refactor/fs-consolidation-part9
Mar 13, 2026
Merged

refactor(ext/node): consolidate node:fs (part 9)#32659
bartlomieju merged 7 commits intodenoland:mainfrom
bartlomieju:refactor/fs-consolidation-part9

Conversation

@bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented Mar 12, 2026

Summary

  • Inlines _fs_readFile.ts and _fs_readlink.ts into the main fs.ts module
  • Updates imports in promises.ts and cp/cp_sync.ts to reference the
    consolidated location
  • Removes 2 file entries from lib.rs
  • Follows the same consolidation pattern as parts 1–8 (refactor(ext/node): consolidate node:fs (part 8) #32640)
  • Skips _fs_read.ts and _fs_readdir.ts for now — they cause circular
    dependency issues when imported from _fs_glob.ts and promises.ts

Test plan

  • cargo build --bin deno passes
  • cargo check -p deno_node passes
  • CI green on node_compat and spec tests

🤖 Generated with Claude Code

bartlomieju and others added 3 commits March 12, 2026 09:50
Inline `_fs_read.ts`, `_fs_readdir.ts`, `_fs_readFile.ts`, and
`_fs_readlink.ts` into the main `fs.ts` module, following the same
pattern as parts 1-8. Update imports in `promises.ts`, `_fs_glob.ts`,
and `cp_sync.ts` to reference the consolidated location.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nto fs.ts

Inline `_fs_readFile.ts` and `_fs_readlink.ts` into the main `fs.ts` module.
Skip `_fs_read.ts` and `_fs_readdir.ts` for now as they cause circular
dependency issues when imported from `_fs_glob.ts` and `promises.ts`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bartlomieju bartlomieju requested a review from Tango992 March 12, 2026 18:06
bartlomieju and others added 2 commits March 12, 2026 22:10
The source snippet frame selection logic skipped `ext:` frames but not
`node:` frames, causing missing source snippets for errors thrown from
node: builtins after the fs consolidation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a drive-by fix because now that code has been moved to node:fs module certain errors were changed, because they didn't match starts_with("ext:").

[Module: null prototype] { sync_js: 1 }
[Module: null prototype] { sync_mjs: 1 }
error: Uncaught (in promise) Error: Top-level await is not allowed in synchronous evaluation
require("./async.js");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is also a consequence of that change 👍

Copy link
Member

@Tango992 Tango992 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bartlomieju bartlomieju merged commit 4da9f0b into denoland:main Mar 13, 2026
112 checks passed
@bartlomieju bartlomieju deleted the refactor/fs-consolidation-part9 branch March 13, 2026 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants